Skip to content

p-token: Add unwrap_lamports instruction #87

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 11 commits into from
Sep 15, 2025
Merged

p-token: Add unwrap_lamports instruction #87

merged 11 commits into from
Sep 15, 2025
+744 −1

Conversation

@febo
Copy link
Contributor

@febo febo commented Sep 2, 2025 •
edited
Loading

Problem

In order to transfer out lamports from native SOL accounts, currently it is necessary to create and close ATAs or token accounts all the time.

Solution

This PR adds a new unwrap_lamports instruction that allows transferring out lamports directly to any destination account. This eliminates the need for creating temporary native token accounts for the recipient.

The new instruction uses the discriminator 3945, which is currently unused in Token-2022.

The amount to unwrap is specified as an Option<u64>:

  • None: the entire token balance is unwrap
  • Some(amount): the specified amount is unwrap

@febo febo requested a review from joncinque September 2, 2025 13:30
@febo febo force-pushed the febo/custom-entrypoint branch 2 times, most recently from fafdd3e to 8a3de38 Compare September 2, 2025 16:08
@febo febo force-pushed the febo/unwrap-lamports branch from 4409253 to 82fa3b5 Compare September 2, 2025 16:09
joncinque
joncinque reviewed Sep 2, 2025
View reviewed changes
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! Just the bit on the discriminator. Also, be sure to get someone like @Arrowana to make sure it does what they need.

@Arrowana
Copy link
Contributor

Arrowana commented Sep 3, 2025 •
edited
Loading

This looks good to me! Just the bit on the discriminator. Also, be sure to get someone like @Arrowana to make sure it does what they need.

It looks ok, but should it have a feature similar to close, allowing unwrapping the entire available amount?

I see a few use cases for this unwrap_lamports:

  • A program which wants to support unwrapping, this PR already works then.
  • User interface / sdk which add convenience, in which case they call a program that might produce a variable amount, so amount is not fixed at execution but it should really unwrap all when they call it.

@febo
Copy link
Contributor Author

febo commented Sep 3, 2025

This looks good to me! Just the bit on the discriminator. Also, be sure to get someone like @Arrowana to make sure it does what they need.

Ir looks ok, but should it have a feature similar to close it allows unwrapping the entire amount?

I see a few use cases for this:

  • A program which wants to support unwrapping, this PR already works then.
  • User interface / sdk which add convenience, in which case they call a program that might produce a variable amount, so amount is not fixed at execution but it should really unwrap all when they call it?

So would it make more sense to always unwrap the entire amount or have both options?

@Arrowana
Copy link
Contributor

Arrowana commented Sep 3, 2025

This looks good to me! Just the bit on the discriminator. Also, be sure to get someone like @Arrowana to make sure it does what they need.

Ir looks ok, but should it have a feature similar to close it allows unwrapping the entire amount?
I see a few use cases for this:

  • A program which wants to support unwrapping, this PR already works then.
  • User interface / sdk which add convenience, in which case they call a program that might produce a variable amount, so amount is not fixed at execution but it should really unwrap all when they call it?

So would it make more sense to always unwrap the entire amount or have both options?

both options

@febo febo requested a review from buffalojoec September 5, 2025 00:13
@febo febo force-pushed the febo/unwrap-lamports branch from b9cdf7c to 369e713 Compare September 5, 2025 00:17
@febo
Copy link
Contributor Author

febo commented Sep 5, 2025

This looks good to me! Just the bit on the discriminator. Also, be sure to get someone like @Arrowana to make sure it does what they need.

Ir looks ok, but should it have a feature similar to close it allows unwrapping the entire amount?
I see a few use cases for this:

  • A program which wants to support unwrapping, this PR already works then.
  • User interface / sdk which add convenience, in which case they call a program that might produce a variable amount, so amount is not fixed at execution but it should really unwrap all when they call it?

So would it make more sense to always unwrap the entire amount or have both options?

both options

@Arrowana PR updated.

buffalojoec
buffalojoec reviewed Sep 9, 2025
View reviewed changes
Copy link
Contributor

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation lgtm, just left a few comments about the tests!

@febo febo force-pushed the febo/unwrap-lamports branch from 369e713 to 49ddb17 Compare September 11, 2025 11:55
@febo febo requested a review from buffalojoec September 11, 2025 11:55
buffalojoec
buffalojoec previously approved these changes Sep 11, 2025
View reviewed changes
Copy link
Contributor

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm from my side!

joncinque
joncinque reviewed Sep 12, 2025
View reviewed changes
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a tiny thing

@febo febo requested a review from joncinque September 12, 2025 23:21
@febo febo force-pushed the febo/unwrap-lamports branch from 599d2b1 to 3b92f39 Compare September 13, 2025 13:53
@febo febo force-pushed the febo/custom-entrypoint branch from f300632 to 2cefb4e Compare September 13, 2025 14:20
@febo febo force-pushed the febo/unwrap-lamports branch from 3b92f39 to 20e0ffe Compare September 13, 2025 14:24
joncinque
joncinque previously approved these changes Sep 15, 2025
View reviewed changes
@febo febo force-pushed the febo/custom-entrypoint branch from 2cefb4e to c68c3d3 Compare September 15, 2025 12:35
Base automatically changed from febo/custom-entrypoint to main September 15, 2025 13:17
@febo febo dismissed stale reviews from joncinque and buffalojoec September 15, 2025 13:17

The base branch was changed.

@febo febo force-pushed the febo/unwrap-lamports branch from 20e0ffe to 4db31a0 Compare September 15, 2025 13:25
@febo febo requested a review from joncinque September 15, 2025 13:37
@febo febo merged commit ae4630e into main Sep 15, 2025
17 checks passed
@febo febo deleted the febo/unwrap-lamports branch September 15, 2025 13:48
@joncinque joncinque mentioned this pull request Oct 8, 2025
Open
@joncinque joncinque mentioned this pull request Nov 4, 2025
Draft
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joncinque joncinque joncinque approved these changes

@buffalojoec buffalojoec buffalojoec left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@febo @Arrowana @joncinque @buffalojoec